Skip to content
This repository has been archived by the owner on Apr 16, 2021. It is now read-only.

go-ipfs on Windows status update #158

Merged
merged 1 commit into from
Apr 24, 2018
Merged

go-ipfs on Windows status update #158

merged 1 commit into from
Apr 24, 2018

Conversation

djdv
Copy link
Contributor

@djdv djdv commented Apr 16, 2018

Here's the initial draft of the work done so far on Windows, open for criticism.
This is intended to be a friendly summary of ipfs/kubo#4808

From my own notes:

  • We should probably find better substitutes for the words "mangled" and "garbage".
  • The "File output names" section doesn't render as expected, it should be reworked
  • Overall English, tone, and typesetting needs work (grammar, image margins, less technical / better humanizing)
  • Windows document isn't merged yet (Change Windows build documentation ipfs/kubo#4691)
  • Better conform to existing posts

Edit:
Rendered: https://ipfs.io/ipfs/QmeXpMzsL2gz5ddYEL1qoPPUEwxoSZqkbPb61DFHQvq796/36-a-look-at-windows/

@ghost ghost added the stage: skeleton label Apr 16, 2018
Copy link
Contributor

@Mr0grog Mr0grog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a few quick thoughts :)

`go-ipfs` is built on top of [Golang](https://golang.org/), which allows for some cross platform compatibility, however it doesn't cover everything. Let's take a look at some of the issues of running `go-ipfs` on Windows and the upcoming solutions for them.

### Log output
![colour](img/colour.png)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be helpful to add a caption just to clarify the top screenshot is what output used to look like and the bottom is what it looks like now. Or split them up and put one before each of these paragraphs.

![garbage](img/garbage.png)

Previously, there was a particularly bad bug that often caused failure for many flatfs (`go-ipfs`'s default storage method) operations, sometimes garbage files would be produced in the working directory as well.
The details of which can be found in the [Windows initiative issue](https://github.com/ipfs/go-ipfs/issues/4808), the short version is that we were trying to move data blocks from a temporary location to a target location, but the target location was being mangled into garbage. {TODO: still sounds too technical; see if we can avoid the word "mangled" as well}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you could simplify this by just stating the issue first, then linking somewhere for more details second:

When trying to move data blocks from a temporary location, the destination path was often mangled. For deeper details, see the Windows initiative issue.

Previously, there was a particularly bad bug that often caused failure for many flatfs (`go-ipfs`'s default storage method) operations, sometimes garbage files would be produced in the working directory as well.
The details of which can be found in the [Windows initiative issue](https://github.com/ipfs/go-ipfs/issues/4808), the short version is that we were trying to move data blocks from a temporary location to a target location, but the target location was being mangled into garbage. {TODO: still sounds too technical; see if we can avoid the word "mangled" as well}

Now, flatfs operations should succeed without the chance of creating garbage files.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would get rid of “the chance of.”

![overwrite 2](img/overwrite%202.png)
{TODO: This is a minor(currently impractical and easily detected) security concern but a security concern nonetheless, consider what tone to use here}

Previously, it was possible for users to craft specific hashes that could escape the extraction root and overwrite files, if the target file's location was known in advanced. In addition, symlinks who's target escaped the hash-root (either via relative or absolute means) were allowed to be created.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

“symlinks who’s” → “symlinks whose”

Now, stdin support has been added to the Windows version of go-ipfs which allows you to place ipfs anywhere in a [pipeline](https://en.wikipedia.org/wiki/Pipeline_(Unix)).

***
In addition to all of the above, there have been many other contributions to `go-ipfs` as well as to the tools it utilizes, both Windows specific and generic. As we move forward, the plan is to continue our efforts of improving and providing a *consistent* experience for `go-ipfs` users.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be correct and useful to link to the Windows initiative issue and say something like:

There’s still a lot more to improve for IPFS on Windows. If you’re interested in contributing (or just keeping an eye on progress), check the Windows initiative issue on go-ipfs.


Now, the building experience should be consistent with other platforms, in addition the [documentation has been rewritten](https://github.com/ipfs/go-ipfs/blob/master/docs/windows.md), clarifying the process and adding a section that covers Windows specific concerns and how to deal with them.

### Access errors / Garbage files {TODO: more appropriate word than "garbage"}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Temporary could work instead of Garbage. The put-* files are essentially this.

Copy link
Contributor

@mishmosh mishmosh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. The descriptions of each improvement are solid.

Let's add (1) more context at the beginning of the post (for the different audiences described in my email, so they can decide whether or not to read this in detail), and (2) beefier follow-up section for questions and contributors. The "Want to contribute?" / "Do you have questions?" sections here are good examples: https://ipfs.io/blog/35-ipfs-companion-2-2-0/

Also some grammar edits inline.

author: Dominic Della Valle
---

`go-ipfs` is built on top of [Golang](https://golang.org/), which allows for some cross platform compatibility, however it doesn't cover everything. Let's take a look at some of the issues of running `go-ipfs` on Windows and the upcoming solutions for them.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Split the first sentence
    • Old: compatibility, however it doesn't
    • New: compatibility. However, it doesn't
  • Regarding "upcoming solutions", are these still upcoming or were they in the latest release? It's a bit at odds with the "Now, xx should..." phrasing below. Let's clarify.
  • Add one more piece of context. Here's a suggestion, feel free to write your own. "Over the past few weeks, we've fine-tuned several aspects of the Windows experience to fix errors and remove inconsistencies."

Previously, the output on Windows was filled with non-native [control characters](https://en.wikipedia.org/wiki/Control_character) which made it hard to read, not only for users but also for developers when users would share their logs for debugging.

![log-after](img/log-after.png)
Now, there should be no more oddities related to character color or cursor placement, text should be clear and lines shouldn't overlap anymore. Which should make everyone a little bit happier.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Which should" makes this a sentence fragment. "That should"?


### Building
![build](img/build.gif)
Previously, there were numerous issues with building the Windows binary, on Windows itself. Silent failures, lack of respect for user supplied arguments, inconsistent handling of dependencies, and others. For a more in-depth look at them, feel free to check out the [Windows initiative issue](https://github.com/ipfs/go-ipfs/issues/4808) which references them.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • "them" ... "them" is blurry. Maybe just delete "at them" and "which references them"?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reference the issue # and name directly (example: "#4808 Windows initiative 2018")

![build](img/build.gif)
Previously, there were numerous issues with building the Windows binary, on Windows itself. Silent failures, lack of respect for user supplied arguments, inconsistent handling of dependencies, and others. For a more in-depth look at them, feel free to check out the [Windows initiative issue](https://github.com/ipfs/go-ipfs/issues/4808) which references them.

Now, the building experience should be consistent with other platforms, in addition the [documentation has been rewritten](https://github.com/ipfs/go-ipfs/blob/master/docs/windows.md), clarifying the process and adding a section that covers Windows specific concerns and how to deal with them.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Run-on sentence. Suggested fix:
"with other platforms. In addition, the documentation has been rewritten to clarify the process and add a section that covers Windows-specific concerns."


### Access errors with temporary files
![garbage](img/garbage.png)
Previously, when trying to move data blocks from a temporary location, the destination path was often mangled. This led to "Access Denied" errors and the creation of garbage files. For deeper details, see the [Windows initiative issue](https://github.com/ipfs/go-ipfs/issues/4808#issuecomment-375796905).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reference the issue # and name directly (example: "#4808 Windows initiative 2018")


***

There’s still a lot more to improve for IPFS on Windows. If you’re interested in contributing (or just keeping an eye on progress), check the [Windows initiative issue](https://github.com/ipfs/go-ipfs/issues/4808) on `go-ipfs`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expand this section a LOT. For reference, the bottom of this blog post from last week is a good example ("Want to contribute?" / "Do you have questions?" sections): https://ipfs.io/blog/35-ipfs-companion-2-2-0/

Copy link
Collaborator

@lidel lidel Apr 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that last week's contribute section is bit browser-centric :) so you may want to look for better "generic" points in earlier posts, eg. https://ipfs.io/blog/33-js-ipfs-0-28/ has:

  • Join an IPFS All Hands, introduce yourself and let us know where you would like to contribute - https://github.com/ipfs/pm/#all-hands-call
  • Hack with IPFS and show us what you made! The All Hands call is also the perfect venue for demos, join in and show us what you built
  • Join the discussion at http://discuss.ipfs.io/ and help users finding their answers.

@djdv
Copy link
Contributor Author

djdv commented Apr 23, 2018

New render: https://ipfs.io/ipfs/QmSvFneeaDn2LsLXH2MV2ZG323DVS9DCCaV39vXZjXjCHt/36-a-look-at-windows/

@mishmosh

Let's add (1) more context at the beginning of the post...

Added a bit to try and keep the attention of non-Windows, non-go-ipfs users.

(2) beefier follow-up section for questions and contributors

Took the footer from the js posts and made it go related.
I wonder if we should have a standard template for this included in the/a skeleton. "go-footer.md", "js-footer.md", etc.

Regarding "upcoming solutions", are these still upcoming or were they in the latest release? It's a bit at odds with the "Now, xx should..." phrasing below. Let's clarify.

I've changed the opening to state that these should be in the next release, as well as rewording the sections themselves to not sound time-relevant. "Now, Previously" -> "Issue:, Resolution:".

Reference the issue # and name directly

I've minimized places where I defer to the issue, it should now only be in the header and right above the footer. But in those places they're styles as such.

There is now a separate issue, linked as
An audit is in progress
Should this be changed to
An audit is in progress (#4485)
or some other form?

@djdv
Copy link
Contributor Author

djdv commented Apr 23, 2018

I didn't touch the "File output names" section, I'm curious about how the markdown is rendered. My editor produces this:
a


ipfs.io produces this:
b

I'm not sure if this is intended or not. Can someone comment on it?
Specifically I mean the list numbers both being 1 and not being formatted as headers.

An ordered list that's bolded may be just as good so I can just rewirte and reformat like that.

@Kubuxu
Copy link
Contributor

Kubuxu commented Apr 23, 2018

Try adding a new line before the list.

@Kubuxu
Copy link
Contributor

Kubuxu commented Apr 23, 2018

Did it work?

@djdv
Copy link
Contributor Author

djdv commented Apr 23, 2018

Current render: https://ipfs.io/ipfs/QmX3sXG3SxvkvH762KQoaSJbBBAnkELyXKKFZtmWjUmYuH/36-a-look-at-windows/

@Kubuxu
It seems so. Thanks.
I tend to get tripped up on those things in markdown. 😅

@ghost ghost assigned mishmosh Apr 23, 2018
@ghost ghost added the status/in-progress In progress label Apr 23, 2018
@mishmosh
Copy link
Contributor

Overall looks good to me. I made a commit to the win branch with 3 small copyedits, your discretion to keep or revert.

@djdv
Copy link
Contributor Author

djdv commented Apr 23, 2018

Edits look good to me. I've reworded one of the sections and cropped an image.
Outside of that I don't think I have any more changes for the post itself.

Is there anything meta that needs to be done for the site itself?
e.g. move/rename files, change url, etc.?

I'm planning on squashing this into 1 commit after an "all clear".

@mishmosh
Copy link
Contributor

@victorbjelkholm or @lidel -- IIRC the publishing process has changed (for the better) since I last did anything blog-related. Can you help @djdv figure out how to publish & any filename/date/etc changes needed?

Thumbs up on the post itself.

@victorb
Copy link
Contributor

victorb commented Apr 24, 2018

Is there anything meta that needs to be done for the site itself? e.g. move/rename files, change url, etc.?

Looks good to me! If all is good, let's merge and ship this :)

@victorb victorb merged commit 443f09d into ipfs-inactive:master Apr 24, 2018
@ghost ghost removed status/in-progress In progress stage: skeleton labels Apr 24, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants